docs(l1): tradeoffs of transactional RocksDB modes #4744
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A brief pros and cons list to decide whether to turn off optimistic transactions.
There was a problem hiding this comment.
Pull Request Overview
Adds an internal documentation page outlining pros/cons of using Optimistic, Pessimistic, or Non-Transactional RocksDB modes to inform a decision about disabling optimistic transactions.
- Introduces advantages sections for each transaction mode plus non-transactional usage
- Lists potential alternatives to using DB-level transactions
- Highlights operational considerations for crash recovery and L2 constraints
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,21 @@ | |||
| # Tradeoffs of using or not Optimistic Transaction DB in RocksDB | |||
There was a problem hiding this comment.
Heading is grammatically awkward; consider rephrasing for clarity.
Suggested change
| # Tradeoffs of using or not Optimistic Transaction DB in RocksDB | |
| # Tradeoffs of Using Optimistic Transaction DB Versus Not Using It in RocksDB |
| - Easier to keep consistency with readers. | ||
|
|
||
| ## Advantages of Pessimistic Transactions | ||
| - Lower memory usage? |
There was a problem hiding this comment.
The trailing question mark signals uncertainty. Either provide data/supporting rationale or remove the entry until confirmed.
Suggested change
| - Lower memory usage? | |
| - Lower memory usage (confirm). |
|
|
||
| ## Advantages of Non-Transactional DB | ||
| - `delete_range`: much faster key deletion in bulk, useful during healing. | ||
| - Lower memory usage (confirm). |
There was a problem hiding this comment.
The '(confirm)' note indicates an unverified claim; replace with concrete information or remove pending validation.
Suggested change
| - Lower memory usage (confirm). | |
| - Lower memory usage. |
| ## Advantages of Non-Transactional DB | ||
| - `delete_range`: much faster key deletion in bulk, useful during healing. | ||
| - Lower memory usage (confirm). | ||
| - Secondary instances are either much easier or possible. |
There was a problem hiding this comment.
Sentence is ambiguous ('either much easier or possible'). Clarify whether non-transactional mode enables secondary instances at all, or simply reduces complexity.
Suggested change
| - Secondary instances are either much easier or possible. | |
| - Secondary instances are only possible in non-transactional mode. |
| - We can provide crash safety by adding checkpoints or explicit markers so we can go back to the last known consistent version. | ||
| - We would need to run a healing pass from that point on startup, to fix possible partial changes to tries. | ||
| - We would need to add explicit tests for crashing during writes to make sure we recover correctly. | ||
| - L2 can't recover from peers (at least until Based), so it should probably remain transactional. |
There was a problem hiding this comment.
Term 'Based' is unexplained; add a brief definition or link so readers understand the dependency or future milestone.
Suggested change
| - L2 can't recover from peers (at least until Based), so it should probably remain transactional. | |
| - L2 can't recover from peers (at least until Based[^based]), so it should probably remain transactional. |
Comment on lines
+12
to
+20
| - `delete_range`: much faster key deletion in bulk, useful during healing. | ||
| - Lower memory usage (confirm). | ||
| - Secondary instances are either much easier or possible. | ||
|
|
||
| ## Alternatives to DB Transactions | ||
| - We can use snapshots/secondary instances to provide a consistent view to block production and protocols. | ||
| - We can provide crash safety by adding checkpoints or explicit markers so we can go back to the last known consistent version. | ||
| - We would need to run a healing pass from that point on startup, to fix possible partial changes to tries. | ||
| - We would need to add explicit tests for crashing during writes to make sure we recover correctly. |
There was a problem hiding this comment.
[nitpick] Consider explicitly listing required additional mechanisms (healing pass, crash tests, checkpoints) as 'Operational Requirements' to distinguish them from 'Advantages'; current mix may confuse readers about what is a benefit vs. a cost.
Suggested change
| - `delete_range`: much faster key deletion in bulk, useful during healing. | |
| - Lower memory usage (confirm). | |
| - Secondary instances are either much easier or possible. | |
| ## Alternatives to DB Transactions | |
| - We can use snapshots/secondary instances to provide a consistent view to block production and protocols. | |
| - We can provide crash safety by adding checkpoints or explicit markers so we can go back to the last known consistent version. | |
| - We would need to run a healing pass from that point on startup, to fix possible partial changes to tries. | |
| - We would need to add explicit tests for crashing during writes to make sure we recover correctly. | |
| - Lower memory usage (confirm). | |
| - Secondary instances are either much easier or possible. | |
| ## Operational Requirements for Non-Transactional DB | |
| - Healing pass: On startup, a healing pass must be run from the last checkpoint to fix possible partial changes to tries. | |
| - Crash safety: Explicit tests for crashing during writes are needed to ensure correct recovery. | |
| - Checkpoints: Crash safety can be provided by adding checkpoints or explicit markers to allow rollback to the last known consistent version. | |
| - `delete_range`: Used for much faster key deletion in bulk, which is especially useful during the healing process. | |
| ## Alternatives to DB Transactions | |
| - We can use snapshots/secondary instances to provide a consistent view to block production and protocols. |
Comment on lines
+17
to
+20
| - We can use snapshots/secondary instances to provide a consistent view to block production and protocols. | ||
| - We can provide crash safety by adding checkpoints or explicit markers so we can go back to the last known consistent version. | ||
| - We would need to run a healing pass from that point on startup, to fix possible partial changes to tries. | ||
| - We would need to add explicit tests for crashing during writes to make sure we recover correctly. |
There was a problem hiding this comment.
[nitpick] Consider explicitly listing required additional mechanisms (healing pass, crash tests, checkpoints) as 'Operational Requirements' to distinguish them from 'Advantages'; current mix may confuse readers about what is a benefit vs. a cost.
Suggested change
| - We can use snapshots/secondary instances to provide a consistent view to block production and protocols. | |
| - We can provide crash safety by adding checkpoints or explicit markers so we can go back to the last known consistent version. | |
| - We would need to run a healing pass from that point on startup, to fix possible partial changes to tries. | |
| - We would need to add explicit tests for crashing during writes to make sure we recover correctly. | |
| - We can use snapshots/secondary instances to provide a consistent view to block production and protocols. | |
| ## Operational Requirements for Non-Transactional DB | |
| - We can provide crash safety by adding checkpoints or explicit markers so we can go back to the last known consistent version. | |
| - We would need to run a healing pass from that point on startup, to fix possible partial changes to tries. | |
| - We would need to add explicit tests for crashing during writes to make sure we recover correctly. |
MegaRedHand
approved these changes
Oct 2, 2025
MegaRedHand
changed the title
mpaulucci
approved these changes
Oct 6, 2025
Peponks9
pushed a commit
to Peponks9/ethrex
that referenced
this pull request
Oct 7, 2025
A brief pros and cons list to decide whether to turn off optimistic transactions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A brief pros and cons list to decide whether to turn off optimistic
transactions.